#9386 closed defect (fixed)
Potential error(e.g., resource leak, deadlock) due to the unreleased lock in function avf_read_packet in FFmpeg/libavdevice/avfoundation.m
Reported by: | cyeaa | Owned by: | |
---|---|---|---|
Priority: | normal | Component: | avdevice |
Version: | git-master | Keywords: | deadlock leak avfoundation |
Cc: | cyeaa | Blocked By: | |
Blocking: | Reproduced by developer: | no | |
Analyzed by developer: | no |
Description
Hi,developers,thank you for your checking.The lock ctx->frame_lock may be not released correctly if the method avf_read_packe returns at eleven error handling branches(line 1067, line 1071, line 1098, line 1104, line 1108, line 1112, line 1131, line 1142, line 1162, line 1172 and line 1174). The relevant code is listed below and the problematic place is commented.
source code link:
https://github.com/FFmpeg/FFmpeg/blob/master/libavdevice/avfoundation.m#L1053
FFmpeg/libavdevice/avfoundation.m:
static void lock_frames(AVFContext* ctx) { pthread_mutex_lock(&ctx->frame_lock); } static void unlock_frames(AVFContext* ctx) { pthread_mutex_unlock(&ctx->frame_lock); } static int avf_read_packet(AVFormatContext *s, AVPacket *pkt) { AVFContext* ctx = (AVFContext*)s->priv_data; do { CVImageBufferRef image_buffer; CMBlockBufferRef block_buffer; lock_frames(ctx); if (ctx->current_frame != nil) { ...; if (image_buffer != nil) { length = (int)CVPixelBufferGetDataSize(image_buffer); } else if (block_buffer != nil) { length = (int)CMBlockBufferGetDataLength(block_buffer); } else { return AVERROR(EINVAL); //line 1067,lock leak: ctx->frame_lock is not released before return } if (av_new_packet(pkt, length) < 0) { return AVERROR(EIO); //line 1071,lock leak: ctx->frame_lock is not released before return } ...; if (status < 0) return status; //line 1098,lock leak: ctx->frame_lock is not released before return } else if (ctx->current_audio_frame != nil) { CMBlockBufferRef block_buffer = CMSampleBufferGetDataBuffer(ctx->current_audio_frame); int block_buffer_size = CMBlockBufferGetDataLength(block_buffer); if (!block_buffer || !block_buffer_size) { return AVERROR(EIO); //line 1104,lock leak: ctx->frame_lock is not released before return } if (ctx->audio_non_interleaved && block_buffer_size > ctx->audio_buffer_size) { return AVERROR_BUFFER_TOO_SMALL; //line 1108,lock leak: ctx->frame_lock is not released before return } if (av_new_packet(pkt, block_buffer_size) < 0) { return AVERROR(EIO); //line 1112,lock leak: ctx->frame_lock is not released before return } ...; if (ctx->audio_non_interleaved) { int sample, c, shift, num_samples; OSStatus ret = CMBlockBufferCopyDataBytes(block_buffer, 0, pkt->size, ctx->audio_buffer); if (ret != kCMBlockBufferNoErr) { return AVERROR(EIO); //line 1131,lock leak: ctx->frame_lock is not released before return } num_samples = pkt->size / (ctx->audio_channels * (ctx->audio_bits_per_sample >> 3)); // transform decoded frame into output format #define INTERLEAVE_OUTPUT(bps) { ...; if (!src) return AVERROR(EIO); //line 1142,lock leak: ctx->frame_lock is not released before return ...; } ...; } else { OSStatus ret = CMBlockBufferCopyDataBytes(block_buffer, 0, pkt->size, pkt->data); if (ret != kCMBlockBufferNoErr) { return AVERROR(EIO); //line 1162,lock leak: ctx->frame_lock is not released before return } } CFRelease(ctx->current_audio_frame); ctx->current_audio_frame = nil; } else { pkt->data = NULL; unlock_frames(ctx); if (ctx->observed_quit) { return AVERROR_EOF; //line 1172,lock leak: ctx->frame_lock is not released before return } else { return AVERROR(EAGAIN); //line 1174,lock leak: ctx->frame_lock is not released before return } } unlock_frames(ctx); } while (!pkt->data); return 0; }
Fixing suggestion:
add unlock_frames(ctx); at all the above branches.
Change History (2)
comment:1 by , 3 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:2 by , 2 years ago
Keywords: | avfoundation added; lock removed |
---|
Note:
See TracTickets
for help on using tickets.
Applied in 54d201ae20d37e1ff5b2fba502fb82e23f090ddb
Thanks!